Skip to content

fix(composer): hide the sandbox control when an external agent is selected - #6704

Open
YongGoose wants to merge 1 commit into
apache:rust-previewfrom
YongGoose:fix/hide-sandbox-for-external-agents
Open

YongGoose wants to merge 1 commit into
apache:rust-previewfrom
YongGoose:fix/hide-sandbox-for-external-agents

Conversation

@YongGoose

Copy link
Copy Markdown
Member

Summary

External CLIs (Claude Code, Codex, agy) run outside Maka's session sandbox, so a conversation's sandbox setting doesn't restrict them. The composer kept showing that setting after an external model was selected, so a conversation could read "Read only" while its model ran with full access.

As agreed in #6702, this hides the sandbox control while the selected backend is the external executor. It applies to an existing conversation and to a new draft, and the control comes back when a native model is chosen. The conversation's stored permissions don't change, and nothing new is enforced. Plugin-provided executors keep the control.

Fixes #6702

Verification

Desktop captures on rust-preview 6fd770539, in a read-only conversation, using the fixture Codex CLI:

Selected model Before After
External model before: Read only is still shown after: the sandbox control is hidden
No model chosen yet (native default) Unchanged after: Read only is still shown

These come from a temporary native desktop test that isn't part of this PR. It skips Desktop::open's text-input wait, because that wait currently times out on rust-preview with observed: Null, the same failure noted on #6700.

  • cargo nextest run --locked -p maka-cli --test integration -E 'test(terminal::external_agents)' passes. The test now checks that the control disappears after the external model is selected and reappears after a native model is selected. Without the Composer.tsx change, it fails at the new check (external_agents.rs:365).
  • The UI typecheck passes: cargo run -p maka-workbench --example ui_types, then tsc -p on the generated config.
  • node scripts/ci.mjs check format passes.

Not run: the full root suite and the Windows and Linux suites.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Claude Code reproduced the issue, wrote the change and the test, captured the screenshots and ran the checks above. The commit carries Generated-by: Claude Code.

Checklist

  • Tests cover the change and fail without it
  • Format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

…ected

External CLIs run outside the Maka session sandbox, so a conversation's
sandbox setting doesn't restrict them. The composer kept showing the
setting after an external model was chosen, which implied that it did.

Hide the control while the selected backend is the external executor,
for an existing conversation and for a new draft. It comes back when a
native model is chosen.

Generated-by: Claude Code

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed both changed files at 4eee7c0ccd8b853e305e0af9b82db23c3c3310be, the existing external-runtime launch settings, the maintainer's direction in #6702, and all three author-provided composer captures.

No confirmed P0–P2 finding. The composer now derives visibility from the active session backend, or the pending draft backend when there is no session. Only executor: external hides the sandbox selector; native/default and plugin-provided executors retain it. The patch changes presentation, not stored permissions or enforcement. The existing Claude/agy launch arguments and Codex sandbox requests substantiate the premise that these external runtimes do not use this session setting.

The actual extracted display predicate passes 20 controlled session/draft backend combinations plus two live backend switches. This uses controlled getters, not a rendered native interface. More directly, current CI job 128658003523 passes external_agents_ui_consent_native_catalog_cache_and_failure_isolation in 40.150s. I read the added real-Host/fixture-runtime assertions: the selector disappears after choosing the external backend and returns after choosing a native model. Its hosted checkout tree matches this reviewed head. The three captures show the external selector removed while the native/default capture retains Read only; they remain the author's images.

This head merges without conflicts into current rust-preview 74765152. Its CI 42456100713 is still a failure: the macOS desktop's first text-input wait fails with observed: Null (67.649s), also failing on its earlier 6fd base. I did not establish this display change as its cause. I did not run real external providers, a local native interface, or an independent native draft interaction. No schema or wire changes.


Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants